Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Virtual envirnoment data dir issues when using local directory #1022

Merged
merged 12 commits into from
Nov 26, 2023

Conversation

apatsekin
Copy link
Contributor

@apatsekin apatsekin commented Oct 31, 2023

Hey guys! I believe there are few issues with how VirtualEnvironment manages directories.
Everything works fine until you try to set HATCH_DATA_DIR=/root_project/.hatch/data - i.e. to something inside the project.

VirtualEnvironment.__init__() checks if data_directory is inside root_directory and based on that either uses "flat structure" or adds a checksum-level of directories.:

 # Conditions requiring a flat structure
elif self.root in self.data_directory.parents or [...]:
    self.storage_path = self.data_directory
    self.virtual_env_path = self.storage_path / venv_name
# Otherwise the defined app path
else:
    self.storage_path = self.data_directory / project_name / checksum
    self.virtual_env_path = self.storage_path / venv_name

But since self.data_directory is not .resolved() it can either be absolute or relative path (both being inside self.root) - based on that if produces different results: HATCH_DATA_DIR=/root_project/.hatch/data - will get a "flat structure" while HATCH_DATA_DIR=.hatch/data- will get a directory with checksum.

While [env]-build environment will always go to checksum directory no matter what.

This behavior brings two issues:

  1. When HATCH_DATA_DIR=/root_project/.hatch/data regular env ends up being in "flat-structure", while -build env goes to "checksum-structure". If user runs hatch build first, it will create env under: root/.hatch/virtual/my-project/jeh852/my-project-build. Then user runs hatch run format and hatch will fail, since it will check for default env under flat structure: root/.hatch/virtual/my-project/ - dir does exist, but there is no env there => hatch will crash on the next step.
  2. While using HATCH_DATA_DIR=.hatch/data (relative path) - the flat structure is completely ignored, which doesn't cause errors, but ignores scenario described here.

nit: when user sets envs.default.path= property only regular environment is created under that path. I guess -build one also should be placed there, right? It's a resource of the same Environment.

TL/DR: proposing PR to properly handle "flat structure", whenHATCH_DATA_DIR is inside root of the project.

@apatsekin
Copy link
Contributor Author

Hey @ofek, would be great if this one makes it to upcoming release.

@ofek
Copy link
Collaborator

ofek commented Nov 22, 2023

Yes it will be thank you!

nit: when user sets envs.default.path= property only regular environment is created under that path. I guess -build one also should be placed there, right? It's a resource of the same Environment.

I want those to always be isolated

@apatsekin
Copy link
Contributor Author

nit: when user sets envs.default.path= property only regular environment is created under that path. I guess -build one also should be placed there, right? It's a resource of the same Environment.

I want those to always be isolated

Yeah they will be isolated, it's just that envs.default.path=/my/path/to/env will be respected by both build and non-build environments:

/my/path/to/env
/my/path/to/env-build

In current master it affects the non-build only, putting -build one to default path with default name:

/my/path/to/env
/default/path/.hatch/env/virtual/[project]/[hash]/[project]-build

which seems to be slightly confusing? If it's intentional, I'm happy to remove this part from PR.

@ofek
Copy link
Collaborator

ofek commented Nov 23, 2023

Yes actually I don't want build environments anywhere else but that internal hidden location away from sight

@apatsekin
Copy link
Contributor Author

Sounds good, I reverted original behavior for envs.default.path. Although -build environment respects HATCH_DATA_DIR, so when it's set to local project dir - all environments go there, right? This is really great feature, since we can configure hatch to create temp files only inside project dir without polluting global disk. But due to -build env ignoring local-dir check, those issues emerge described in the PR. So I changed it to minimal:

  1. add a check for -build environment to use flat structure when necessary, based on isolated_data_directory
  2. use .resolve() to check if HATCH_DATA_DIR is actually inside project root.

Copy link
Collaborator

@ofek ofek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ofek ofek merged commit 0326e47 into pypa:master Nov 26, 2023
3 checks passed
github-actions bot pushed a commit that referenced this pull request Nov 26, 2023
* fixing virtual env directories

* black

* checking isolated_data_directory

* checking isolated_data_directory

* signed commit

* fast path

* add release note

* spacing

---------

Co-authored-by: Ofek Lev <[email protected]> 0326e47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants